feat: add evm provider and logger #10
Conversation
packages/shared/src/external.ts
Outdated
There was a problem hiding this comment.
missing the export of the Logger class
| } | ||
|
|
||
| this.client = createPublicClient({ | ||
| chain, |
There was a problem hiding this comment.
optional: do we need any validation of the chain? It's completely optional here?
There was a problem hiding this comment.
maybe we should add a validation to ensure that multicall works, thats the only thing that conserns me, just to avoid throwing in the case multicall address doesn't exist. Or maybe providing another way as fallback mechanism like a Promise.all in case there is no multicall contract (we already talk about the last one with nigiri).
Lets move forward as it is and keep track of this in linear, to tackle the issue in the future.
| }); | ||
|
|
||
| describe("getGasPrice", () => { | ||
| it("should return the current gas price", async () => { |
There was a problem hiding this comment.
This is picky but I think we are supposed to avoid writing "should" for every it() statement and instead just say "returns.."
There was a problem hiding this comment.
you are right, will take this to fix during future pr :)
| * @returns {Promise<bigint>} A Promise that resolves to the latest block number. | ||
| */ | ||
| async getBlockNumber(): Promise<bigint> { | ||
| return this.client.getBlockNumber(); |
There was a problem hiding this comment.
Pretty convoluted but just wanted to tell that getBlockNumber by default caches the result by 4 seconds (ie default client's polling interval). [1]
Not sure if this is relevant but might be.
[1] https://viem.sh/docs/actions/public/getBlockNumber.html#cachetime-optional
There was a problem hiding this comment.
not sure if this is that relevant, we typically have all the events linked to the blocknumbers where they were emitted
cc @0xnigir1 ??
There was a problem hiding this comment.
i think it's unlinkely we have an issue with this, its not that we call this method for every event. anyway, lets keep an eye an eventually we set it to 0
| async getBlockByNumber(blockNumber: number): Promise<GetBlockReturnType> { | ||
| return this.client.getBlock({ blockNumber: BigInt(blockNumber) }); | ||
| } |
There was a problem hiding this comment.
Any specific reason to use number over bigint here? With this interface, someone could pass blockNumber = 5.5 as an argument and BigInt(blockNumber) would explode.
|
There is a bug in the UI and can't be merged |
Description
chain-providerssharedtestincheck-typesscriptpricingtestsChecklist before requesting a review